Skip to content

[lockfile-explorer] Architecture improvents#5357

Merged
octogonz merged 21 commits intomicrosoft:mainfrom
octogonz:octogonz/lfx-fixes
Sep 14, 2025
Merged

[lockfile-explorer] Architecture improvents#5357
octogonz merged 21 commits intomicrosoft:mainfrom
octogonz:octogonz/lfx-fixes

Conversation

@octogonz
Copy link
Collaborator

@octogonz octogonz commented Sep 13, 2025

Summary

This is a refactoring step to prepare for future improvements of Lockfile Explorer.

Details

Lockfile Explorer is separated into a Node.js server and a web browser client. Today the YAML parsing is split between client/server, which makes it difficult to support and debug lockfile format versions. This PR is the first step towards a new architecture where the server will do all the YAML parsing to produce the "graph" that backs the client's UX. That means we need to isolate the graph data and make it JSON-serializable.

We also need to move lfxGraphLoader.ts to run on the server instead of client, but I will do that as a separate PR.

Summary of changes in this PR:

  • Isolate the serializable model in LfxGraph.ts
  • Extract the YAML parsing and graph construction in lfxGraphLoader.ts
  • Define a JSON serializable contract in JsonLfxGraph.ts
  • Implement a serializer/deserializer in lfxGraphSerializer.ts
  • Add a REST API for communicating workspace settings to the client
  • Improve some project settings so devServer and IntelliSense work correctly

How it was tested

Impacted documentation

None

@william2958

@@ -0,0 +1,363 @@
// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
// See LICENSE in the project root for license information.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once this code is moved to Node.js, we can start cleaning it up and improving the support for different PNPM lockfile versions.

From the spec:

Relation of pnpm version to lockfile version

lockfile version used by pnpm versions
9.0 >=9.0.0
6.0 >=8.0.0 <9.0.0
5.4 >=7.0.0 <8.0.0
5.3 >=6.0.0
. . . . . .

"eslint": "~9.25.1",
"local-web-rig": "workspace:*"
"local-web-rig": "workspace:*",
"typescript": "5.8.2"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

Copy link
Member

@iclanton iclanton Sep 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That isn't a problem in other Rush repos. VSCode should be able to fall back to TypeScript that it ships with.

Comment on lines +6 to +11
declare global {
// eslint-disable-next-line @typescript-eslint/naming-convention
interface Window {
appContext: IAppContext;
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
declare global {
// eslint-disable-next-line @typescript-eslint/naming-convention
interface Window {
appContext: IAppContext;
}
}
declare const window: Window & { appContext: IAppContext };

Copy link
Collaborator Author

@octogonz octogonz Sep 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iclanton Your version didn't compile:

Encountered 5 errors
  [build:typescript] src/containers/LogoPanel/index.tsx:10:44 - (TS2339) Property 'appContext' does not exist on type 'Window & typeof globalThis'.
  [build:typescript] src/helpers/lfxApiClient.ts:7:36 - (TS2339) Property 'appContext' does not exist on type 'Window & typeof globalThis'.
  [build:typescript] src/helpers/logDiagnosticInfo.ts:7:14 - (TS2339) Property 'appContext' does not exist on type 'Window & typeof globalThis'.
  [build:typescript] src/parsing/readLockfile.ts:7:35 - (TS2339) Property 'appContext' does not exist on type 'Window & typeof globalThis'.
  [build:typescript] src/stub/initappcontext.ts:11:8 - (TS2339) Property 'appContext' does not exist on type 'Window & typeof globalThis'.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd probabl have to be /// <reference ... />'d

"changes": [
{
"packageName": "@rushstack/lockfile-explorer",
"comment": "Refactoring to support future work",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this useful in the changelog?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be if we automatically publish a patch change and it breaks something.

@octogonz octogonz merged commit 39a0b6b into microsoft:main Sep 14, 2025
5 checks passed
@github-project-automation github-project-automation bot moved this from Needs triage to Closed in Bug Triage Sep 14, 2025
@octogonz octogonz deleted the octogonz/lfx-fixes branch September 14, 2025 02:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Closed

Development

Successfully merging this pull request may close these issues.

2 participants